-
Notifications
You must be signed in to change notification settings - Fork 125
Add HTTPClientError shortDescription property #583
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add HTTPClientError shortDescription property #583
Conversation
Can one of the admins verify this patch? |
6 similar comments
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
@@ -949,6 +949,73 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible { | |||
return "HTTPClientError.\(String(describing: self.code))" | |||
} | |||
|
|||
public var shortDescription: String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind adding a comment that describes, when users should use shortDescription
and what they need to take into consideration when adding another case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added now. Happy to adjust the comments of course!
3a08807
to
0becaf9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small change :) Since this is a public API change, let's get @Lukasa on here as well.
@@ -949,6 +949,76 @@ public struct HTTPClientError: Error, Equatable, CustomStringConvertible { | |||
return "HTTPClientError.\(String(describing: self.code))" | |||
} | |||
|
|||
/// Short description of the error that can be used in case a bounded set of error descriptions is expected, e.g. to | |||
/// include in metric labels. The description does not contain associated values. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// include in metric labels. The description does not contain associated values. | |
/// include in metric labels. For this reason the description must not contain associated values. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. Do you still want to keep the in-code comment two lines down, saying essentially the same?
This adds a shortDescription property to HTTPClientError which provides a short description of the error without associated values.
0becaf9
to
5b0496f
Compare
@swift-server-bot test this please |
@swift-server-bot add to allowlist |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks! Let's wait for @Lukasa thumb though.
This adds a shortDescription property to HTTPClientError which provides a short
description of the error without associated values.
This is useful for e.g. metric labels which should not contain dynamic, potentially unbounded, associated values.